-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RNMobile] Avoid keyboard dismiss when interacting text blocks #57070
[RNMobile] Avoid keyboard dismiss when interacting text blocks #57070
Conversation
Size Change: -762 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
Flaky tests detected in 17dcda5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7221803754
|
This function will help us to deal with the special case of unfocusing an Aztec view upon unmounting.
This was previously handled in the `RichText` component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This is a major improvement to the writing flow experience 🤩
The code changes look good to me, I'm happy that we were able to achieve this with not that many code changes or refactors 🚀
I've tested all test cases scenarios including the writing flow without any issues or regressions.
Devices used:
- iPhone 15 Pro (OS 17.1.2)
- Samsung Galaxy A23 5G (OS Android 13)
- Redmi Note 8T (OS Android 11)
Really nice work! 👏
I think to simplify things we could merge #57069 into this branch and then merge this one into |
) * React Native Bridge - Android - Introduces showAndroidSoftKeyboard to show the keyboard if there's a focused TextInput * Mobile - Bottom Sheet - Adds usage of showAndroidSoftKeyboard when closing the Modal so it shows the Keyboard on Android for focused TextInputs * React Native Bridge - Android - Introduces hideAndroidSoftKeyboard to hide the keyboard without triggering blur events * React Native Bridge - Remove console warnings for unsupported methods, as their names are self-explanatory. * Update showAndroidSoftKeyboard to take into account when the window focus changed, when we show the Modals these are shown on top of the editor activity. It also adds an option to delay this for full screen modals * Mobile - BottomSheet - Enable hardwareAccelerated and useNativeDriverForBackdrop props to improve performance on Android * Update snapshots * Removes hasWindowFocus condition as it is not being called hence not needed * Refactor showAndroidSoftKeyboard to split into several functions, it also removes the delay functionality as it is no longer needed. It fixes an issue where mKeyboardRunnable was not being set. It removes the delay logic from the Bottom Sheet component and the bridge. * Updates createShowKeyboardRunnable to get the activity within the runnable instead of getting it as an param * Remove unneeded check
The changes from #57069 are now merged into this branch 🎉 |
Before heading to merge the PR, I'll update the changelog to reflect the fixes. |
Hi @fluiddot, I ran into a crash on Android from these changes. Screen: screen-20231226-171440.mp4I get the error from anywhere on the post with or without any existing content. I'm running the app locally on a Pixel 8 with WP-Android checked out at wordpress-mobile/WordPress-Android@70eb2d9 . I don't get the error after reverting the PR merge commit. |
Hey @jhnstn 👋 I'll investigate this issue, thanks for reporting! |
@jhnstn This issue is happening because we haven't released an alpha version of Gutenberg Mobile and these changes include native code. If you build Gutenberg Mobile from source it should fix it, you can do this by changing your local Let us know if this fixes the problem until we create an alpha tag. |
Thanks @geriux ! setting up the local GB path fixed it |
* Use debounce in Aztec's blur function * Execute `focus` UI block before other blocks * Add `hideAndroidSoftKeyboard` to RN bridge * Add `blurOnUnmount` to Aztec input state manager. This function will help us to deal with the special case of unfocusing an Aztec view upon unmounting. * Dismiss keyboard when Aztec view unmounts This was previously handled in the `RichText` component. * Fix unit test related to `AztecInputState` after adding debounce to `blur` function * Remove console warning from `hideAndroidSoftKeyboard` * Update inline comments of `blurOnUnmount` function * [Mobile] - Android - Bring the Keyboard back when closing modals (#57069) * React Native Bridge - Android - Introduces showAndroidSoftKeyboard to show the keyboard if there's a focused TextInput * Mobile - Bottom Sheet - Adds usage of showAndroidSoftKeyboard when closing the Modal so it shows the Keyboard on Android for focused TextInputs * React Native Bridge - Android - Introduces hideAndroidSoftKeyboard to hide the keyboard without triggering blur events * React Native Bridge - Remove console warnings for unsupported methods, as their names are self-explanatory. * Update showAndroidSoftKeyboard to take into account when the window focus changed, when we show the Modals these are shown on top of the editor activity. It also adds an option to delay this for full screen modals * Mobile - BottomSheet - Enable hardwareAccelerated and useNativeDriverForBackdrop props to improve performance on Android * Update snapshots * Removes hasWindowFocus condition as it is not being called hence not needed * Refactor showAndroidSoftKeyboard to split into several functions, it also removes the delay functionality as it is no longer needed. It fixes an issue where mKeyboardRunnable was not being set. It removes the delay logic from the Bottom Sheet component and the bridge. * Updates createShowKeyboardRunnable to get the activity within the runnable instead of getting it as an param * Remove unneeded check * Update `react-native-editor` changelog --------- Co-authored-by: Gerardo Pacheco <gerardo.pacheco@automattic.com>
Related PRs:
What?
This PR solves the following cases that are affected by the keyboard’s disruptive hide-and-show behavior:
enter
key in a text-based block.backspace
key in a text-based block.However, there's a remaining case that this PR won't cover:
We haven't found a solution to this yet, as a next step, we'd need to explore further the actions performed during block creation and merging in the List block.
Why?
This PR will greatly improve the usability when writing content, as the hide-and-show behavior impacts negatively the editing experience.
How?
Solve
blur
/focus
event conflictsEach
RichtText
component instance triggersblur
/focus
events based on its state (e.g. will be unmounted, gains/loses selection) without considering other instances states. This leads to conflicts betweenblur
andfocus
events in different cases like pressingenter
on a Paragraph block to insert a new paragraph.Here is an example of the sequence of events generated when pressing
enter
:blur
event. This comes from thecomponentDidUpdate
function due to changing its selection state from selected to unselected.blur
is the culprit of the keyboard being dismissed, as it executed first.focus
event when it's mounted (i.e. from thecomponentDidMount
function).focus
event makes the keyboard to open.focus
a second time. However, this event is innocuous. This comes from thecomponentDidUpdate
function due to changing its selection state from unselected to selected.blur
event. This comes from the_onBlur
callback triggered by Aztec because the text input lost focus. This event is innocuous.To solve this conflict, this PR introduces the usage of
debounce
in theblur
event (b6f237f). It will ensure thatfocus
events take precedence whenblur
events have been triggered at the same time, as well as cancel them to avoid the keyboard to be dismissed.iOS - Keyboard dismisses automatically when text input is removed
Another case to solve is when two blocks are merged. The editor internally replaces the blocks which implies removing one of them. On iOS, this presents a problem because the keyboard will be automatically dismissed. No
blur
events are generated so a different approach was taken.The solution (a71b1c5) implied tweaking the order of native commands invoked for
blur
andfocus
events (reference). Thefocus
event triggered by the new block created (i.e. the one that contains the merged content) happens at the same time the other block is removed. So, the workaround for this case is to ensure thatfocus
events are prepended in the pending UI blocks list of theUIManager
(reference 1 , reference 2).Android - Avoid triggering
blur
events on unmounted instancesAs mentioned below, when
RichText
/AztecView
is unmounted ablur
event is triggered. On Android, this behavior in combination with usingdebounce
leads to an exception because we try toblur
a text input that no longer exists. To address this, a new function has been introduced in the Aztec input state (blurOnUnmount
) that is only called for the component unmounting case (634867f). This function has two purposes:blur
calls to avoid exceptions.blur
event. Note that the call to hide the keyboard usesdebounce
. This is needed to avoid dismissing the keyboard if afocus
event happens at the same time.Other changes
As a side note, the
blur
event triggered when unmounting theRichText
component has been moved to theAztecView
component as seems a better fit for this logic (d3fb486).Testing Instructions
Note
It's recommended to use the installable builds for testing:
1 - Post title
2 - Insert a new block by pressing the
enter
key in a text-based block3 - Merge blocks by pressing the
backspace
key in a text-based block (iOS only)4 - Automatically dismiss the keyboard when selecting non-text-based-blocks
5 - Manually dismiss the keyboard
6 - The keyboard is dismissed and restored when opening modals
NOTE: On Android, this case will be covered by #57069.
6 - Writing flow
It's recommended to cover the writing flow test suite by manually testing.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
ios-avoid-keyboard-dismiss-when-interacting-text-blocks.MP4
android-avoid-keyboard-dismiss-when-interacting-text-blocks.mp4